Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(anta): Mcs client mounts #898

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

sarunac
Copy link
Contributor

@sarunac sarunac commented Oct 24, 2024

Description

Anta Tests to verify if management cvx mounts for service mcs is in healthy state = mountStateMountComplete

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have run pre-commit for code linting and typing (pre-commit run)
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes (tox -e testenv)

Copy link

codspeed-hq bot commented Oct 24, 2024

CodSpeed Performance Report

Merging #898 will not alter performance

Comparing sarunac:mcsClientMounts (be45bd2) with main (a8aeb44)

Summary

✅ 8 untouched benchmarks

Copy link

sonarcloud bot commented Oct 24, 2024

"""

name = "VerifyMcsClientMounts"
description = "Verify if all service MCS Client management cvx mounts are mountStateMountComplete"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we try to keep the description shorter here as they show up in the reports and it can clutter them

Comment on lines +141 to +142
* Success: The test will pass if the MCS mount status on MCS Clients are mountStateMountComplete.
* Failure: The test will fail if the MCS mount status on MCS Clients are not mountStateMountComplete.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs rewording as there are several failure tests and I am not sure what to expect if all clients are mounted or if only one is?

mcs_mount_state_detected = True
state = mount_state["state"]
if state != "mountStateMountComplete":
self.result.is_failure(f"MCS Client mount states are not valid: {state}")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this enough information in the error message? would you want to the the type that's failing as well?

Comment on lines +165 to +169
if mount_state["type"].startswith("Mcs"):
mcs_mount_state_detected = True
state = mount_state["state"]
if state != "mountStateMountComplete":
self.result.is_failure(f"MCS Client mount states are not valid: {state}")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if mount_state["type"].startswith("Mcs"):
mcs_mount_state_detected = True
state = mount_state["state"]
if state != "mountStateMountComplete":
self.result.is_failure(f"MCS Client mount states are not valid: {state}")
if not mount_state["type"].startswith("Mcs"):
continue
mcs_mount_state_detected = True
if (state := mount_state["state"]) != "mountStateMountComplete":
self.result.is_failure(f"MCS Client mount states are not valid: {state}")

if state != "mountStateMountComplete":
self.result.is_failure(f"MCS Client mount states are not valid: {state}")

if len(mount_states) == 0 or not mcs_mount_state_detected:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is redundant as if len(mount_states) == 0 then necessarily mcs_mount_state_detected will be False

Suggested change
if len(mount_states) == 0 or not mcs_mount_state_detected:
if not mcs_mount_state_detected:

Examples
--------
```yaml
anta.tests.configuration:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it really a configuration test?

Would you not prefer a new MCS category maybe?

cc @carl-baillargeon

Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants